de-mystify resource internals #1166
Conversation
|
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Estimated impact to a consuming app, depending on which bundle is imported
|
is thish better? maybe tests will tell me ope ope The builder had to be registered to the helper manager Be more robust with owner detection Maybe progress it works, now for memory leak testing
NullVoxPopuli
force-pushed
the
make-the-return-the-value-public-api
branch
from
8e74bcd to
2400805
Compare
| test('a generated interval can be cleared', async function (assert) { | ||
| const id = cell(0); | ||
| const condition = cell(true); | ||
|
|
||
| const poll = resourceFactory((id: number) => { |
There was a problem hiding this comment.
resourceFactory isn't needed here, and is kinda awkward to have inline like this (moreso that the call below, imo)
|
Review from devin.ai: PR Review: ember-resources #1166 "de-mystify resource internals"SummaryThis PR successfully transforms the internal resource mechanics into a more explicit, transparent public API through a new Builder/Resource pattern. The changes maintain backward compatibility while making the implementation more accessible to developers. Key Changes
Code Quality Assessment
RecommendationApprove and Merge the PR. The changes successfully "de-mystify" resource internals by making them more explicit and transparent, which will benefit developers working with the library. All CI checks are passing, and the code quality is high. The PR achieves its stated goal of making the resource API less magical and more developer-friendly while maintaining backward compatibility. |
Devin 1:18 PM The PR has a few minor type definition issues (marked with @ts-expect-error in tests) and a TODO comment in Resource class that could be completed, but no significant issues. |
No description provided.